Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backend/rs: delay client cleanup #772

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

cmeissl
Copy link
Collaborator

@cmeissl cmeissl commented Dec 14, 2024

delay client cleanup until the state lock is dropped to prevent potential deadlocks in drop impls of the client state.

delay client cleanup until the state lock is dropped
to prevent potential deadlocks in drop impls of the
client state.
@cmeissl cmeissl marked this pull request as ready for review December 14, 2024 21:15
@cmeissl cmeissl force-pushed the fix/client_cleanup_deadlock branch from 3cef0cf to b07d3f3 Compare December 14, 2024 21:18
@cmeissl cmeissl force-pushed the fix/client_cleanup_deadlock branch from 2ff8a99 to 50bb662 Compare December 14, 2024 22:48
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.08%. Comparing base (3375b59) to head (50bb662).
Report is 23 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #772      +/-   ##
==========================================
+ Coverage   72.54%   74.08%   +1.54%     
==========================================
  Files          41       46       +5     
  Lines        6516     7757    +1241     
==========================================
+ Hits         4727     5747    +1020     
- Misses       1789     2010     +221     
Flag Coverage Δ
main 59.36% <100.00%> (?)
test-- 78.21% <100.00%> (?)
test--server_system 61.30% <0.00%> (?)
test-client_system- 69.48% <100.00%> (-3.07%) ⬇️
test-client_system-server_system 51.94% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ids1024 ids1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a reasonable way to avoid the deadlock here.

@ids1024 ids1024 merged commit 6049eab into Smithay:master Dec 17, 2024
15 checks passed
@cmeissl cmeissl deleted the fix/client_cleanup_deadlock branch December 18, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants